Conversation
📝 WalkthroughRelease Notes - Adaptive Navigation Patterns (PR
|
| Cohort / File(s) | Summary |
|---|---|
Toolbar Configuration Simplification app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt |
Removed syncProjectToolbarRowForOrientation and its calls from onCreate/onConfigurationChanged; removed unused LinearLayout/TextView imports. |
Template List Adapter Simplification app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.kt |
Removed DiffUtil-based diffing and fillDiff logic; adjusted long-press tooltip lambda parameter naming. |
Template List Layout Restructuring app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt |
Replaced Flexbox layout with GridLayoutManager (initial span=1); added onConfigurationChanged override and updateSpanCount() to recompute spanCount based on orientation/item count; removed global layout listener and Flexbox utilities. |
Editor AppBar UI Change app/src/main/res/layout/content_editor.xml |
Replaced standalone toolbar with a FlexboxLayout containing the title toolbar and a new ProjectActionsToolbar; adjusted sizing, margins, and wrapping behavior. |
Project Actions Toolbar Width Adjustment common/src/main/res/layout/project_actions_toolbar.xml |
Changed layout_width from match_parent to wrap_content on MaterialToolbar and its HorizontalScrollView. |
Template Item Width app/src/main/res/layout/layout_template_list_item.xml |
Changed MaterialCardView and inner LinearLayout width from wrap_content to match_parent for full-width items. |
Sequence Diagram(s)
(omitted)
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- ADFA-2328 Expand the ribbon slider to a new position and make it as wide as the app #755: Modifies editor toolbar layout/initialization and ProjectActionsToolbar usage — strong overlap with AppBar/layout changes.
- ADFA-77 Enable landscape mode #993: Adds/removes orientation-handling for BaseEditorActivity toolbar; directly conflicts with toolbar-orientation removals here.
- fix(ADFA-3093): Add bottom padding to prevent overlap on system navigation bar #1021: Changes TemplateListFragment insets/layout handling; related to template list layout/insets adjustments.
Suggested reviewers
- itsaky-adfa
- jomen-adfa
- hal-eisen-adfa
Poem
🐰 I hopped through layouts, trimmed a twisty rule,
I nudged the grid to make the list feel cool.
Toolbars now bask without turning round,
Fewer hops, lighter paws upon this ground. 🥕✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The PR title 'ADFA-3123 | Adaptive navigation patterns' directly captures the main objective of refactoring UI for adaptive layout support across screen sizes and orientations. |
| Description check | ✅ Passed | The PR description thoroughly explains all major changes including editor toolbars, template list migration, template items, and cleanup—all of which are present in the changeset. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/ADFA-3123-adaptive-navigation-patterns
Tip
Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt`:
- Around line 79-84: The tooltip is being anchored to binding.root inside the
setOnLongClickListener attached to binding.exitButton, which can position it far
from the pressed control; update the call to
TooltipManager.showIdeCategoryTooltip (inside the
binding.exitButton.setOnLongClickListener block) to use the exit button as the
anchor (e.g., anchorView = binding.exitButton or the listener's view parameter)
while keeping context=requireContext() and tag=EXIT_TO_MAIN so the tooltip
appears next to the long-pressed button.
- Around line 68-73: The span calculation currently uses
resources.configuration.screenWidthDp; change to compute span count from the
actual RecyclerView width by adding a helper method (e.g.,
updateSpanCount(minItemWidthDp: Int)) in TemplateListFragment that measures
binding.list.width (after layout) and sets gridLayoutManager.spanCount
accordingly, call this helper from binding.list.doOnLayout for initial setup and
from onConfigurationChanged() to recompute on rotations/size changes; ensure you
keep the GridLayoutManager instance (gridLayoutManager) and update its spanCount
rather than recreating the adapter or list.
In `@app/src/main/res/layout/content_editor.xml`:
- Around line 65-74: The ProjectActionsToolbar is declared wrap_content but
inflates project_actions_toolbar.xml whose root MaterialToolbar uses
android:layout_width="match_parent", causing measurement ambiguity; open
project_actions_toolbar.xml and change the root MaterialToolbar's
android:layout_width to "wrap_content" (and likewise change any immediate child
views that use match_parent to wrap_content or use wrap_content with appropriate
paddings) so the custom view ProjectActionsToolbar can size itself correctly
within the FlexboxLayout.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c2c5a9c-d4dd-4eef-8f67-57efbc14f805
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktapp/src/main/res/layout/content_editor.xmlapp/src/main/res/layout/layout_template_list_item.xml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt
Show resolved
Hide resolved
Change toolbar to wrap_content and cap grid columns by orientation and item count.
b66cbf0 to
290fe77
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt (1)
99-111: Consider using actual RecyclerView width for span calculation.The current approach uses orientation and item count, but doesn't account for the actual available width of
binding.list. This can cause issues in constrained layouts (split-screen, foldables, embedded containers) where the RecyclerView may be narrower than expected.Using
binding.list.widthafter layout would provide more accurate span counts across all scenarios.Suggested approach using actual width
+import androidx.core.view.doOnLayout ... + private val minItemWidthDp = 160 + override fun onConfigurationChanged(newConfig: Configuration) { super.onConfigurationChanged(newConfig) - - updateSpanCount() + _binding?.list?.post { updateSpanCount() } } private fun updateSpanCount() { - val isLandscape = resources.configuration.orientation == Configuration.ORIENTATION_LANDSCAPE - val maxSpans = if (isLandscape) 6 else 4 - + val listWidthPx = binding.list.width + if (listWidthPx <= 0) return + + val minItemWidthPx = (minItemWidthDp * resources.displayMetrics.density).toInt() val itemCount = binding.list.adapter?.itemCount ?: 0 - - val optimalSpans = maxOf(1, minOf(maxSpans, itemCount)) - + val widthBasedSpans = (listWidthPx / minItemWidthPx).coerceIn(1, 6) + val optimalSpans = minOf(widthBasedSpans, maxOf(1, itemCount)) + val layoutManager = binding.list.layoutManager as? GridLayoutManager if (layoutManager != null && layoutManager.spanCount != optimalSpans) { layoutManager.spanCount = optimalSpans } }Also update the initial setup to use
doOnLayout:binding.list.layoutManager = gridLayoutManager binding.list.doOnLayout { updateSpanCount() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt` around lines 99 - 111, The updateSpanCount currently bases spans on orientation and item count only; change it to measure the actual available width of binding.list (use binding.list.width after layout) and compute optimalSpans from availableWidth divided by a target minimum column width (e.g., desiredItemMinWidth), clamped between 1 and maxSpans; update the GridLayoutManager.spanCount accordingly in updateSpanCount; also ensure updateSpanCount is invoked after layout by calling binding.list.doOnLayout { updateSpanCount() } (and/or attach an OnLayoutChangeListener) where the layout manager is set so width measurement is valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt`:
- Around line 99-111: The updateSpanCount currently bases spans on orientation
and item count only; change it to measure the actual available width of
binding.list (use binding.list.width after layout) and compute optimalSpans from
availableWidth divided by a target minimum column width (e.g.,
desiredItemMinWidth), clamped between 1 and maxSpans; update the
GridLayoutManager.spanCount accordingly in updateSpanCount; also ensure
updateSpanCount is invoked after layout by calling binding.list.doOnLayout {
updateSpanCount() } (and/or attach an OnLayoutChangeListener) where the layout
manager is set so width measurement is valid.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46fe4dd9-d078-4c5b-9a0b-dd6e86b9cacd
📒 Files selected for processing (6)
app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.ktapp/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktapp/src/main/res/layout/content_editor.xmlapp/src/main/res/layout/layout_template_list_item.xmlcommon/src/main/res/layout/project_actions_toolbar.xml
💤 Files with no reviewable changes (1)
- app/src/main/java/com/itsaky/androidide/activities/editor/BaseEditorActivity.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/layout/layout_template_list_item.xml
This PR refactors the UI to properly support adaptive navigation patterns across different screen sizes and orientations, specifically addressing the editor's toolbars and the project template list.
Changes include:
BaseEditorActivitywith a nativeFlexboxLayoutincontent_editor.xml. This automatically handles wrapping theProjectActionsToolbarbelow thetitle_toolbaron smaller screens and places them side-by-side on larger screens (like tablets or in landscape mode), eliminating the need for complex layout calculations in code.FlexboxLayoutManagerwith a customGlobalLayoutListenerto a dynamicGridLayoutManagerinTemplateListFragment. This natively adjusts the number of columns (between 1 and 4) based on the available screen width, providing a much smoother and more performant reflow when resizing the window (e.g., in DeX mode) or rotating the device.layout_template_list_item.xmlto usematch_parentwidths, ensuring the cards properly fill their assigned grid columns without leaving awkward empty spaces.TemplateListAdapter.Details
1000097917.mp4
Ticket
ADFA-3123
Observation
The use of
GridLayoutManagercombined withonConfigurationChangedfor the template list provides a significant performance improvement over the previousGlobalLayoutListenerapproach, especially in desktop-like environments (Samsung DeX) where window resizing triggers frequent layout passes.